Skip to content

ci(solidity): clear the remaining type errors and gate tsc in CI - #4207

Merged
piotr-roslaniec merged 4 commits into
devfrom
chore/solidity-typecheck-gate
Sep 12, 2026
Merged

ci(solidity): clear the remaining type errors and gate tsc in CI#4207
piotr-roslaniec merged 4 commits into
devfrom
chore/solidity-typecheck-gate

Conversation

@mswilkison

Copy link
Copy Markdown
Contributor

Stacked on #4206. The tail of the Node 24 chain: with TypeScript raised to 5.9.3, both solidity packages can be brought to zero tsc errors and held there.

Nothing in CI ran the type checker. yarn lint is eslint, and hardhat transpiles tests without checking them, so a type error only surfaced if someone ran tsc by hand.

What was broken

Package Errors Where
random-beacon 21 all in generated typechain/
ecdsa 1 test/WalletRegistry.RandomBeacon.test.ts

random-beacon — every error was TS7018 in generated factories. random-beacon/tsconfig.json sets noImplicitAny: true, and typechain 7 emits ABI literals as bare const _abi = [{ inputs: [], ... }]; under that flag an empty array literal infers any[]. So the package was type-checking generated output more strictly than its generator supported, in files nobody can edit.

typechain 8 emits the same literals as const and the class disappears. The alternative — dropping noImplicitAny — would have bought a clean run by checking our own code less, so it is not what this does.

Two notes on the bump:

  • @typechain/hardhat is pinned to ^7, not the current ^9, because 9 defaults to the ethers-v6 target and this package is still on v5.
  • v8 nests generated modules under their source path instead of emitting flat files, so five deep imports move. Four are Solidity library namespaces (Groups, BeaconDkg) which still have to come from the module of a contract that uses them; the fifth, RandomBeaconGovernance, is an ordinary contract type and now comes from the barrel with its neighbours.

ecdsa — one real bug. submitRelayEntry takes bytes, and the test passed a BigNumber built from 32 random bytes. Beyond not being BytesLike, ethers renders a BigNumber argument through toHexString(), which drops leading zero bytes — so roughly one run in 256 submitted an entry shorter than 32 bytes. The stub only keccak-hashes the value, so nothing failed and nothing noticed. The random bytes are already a Uint8Array, so they now go in directly.

The gate

yarn typecheck (tsc --noEmit -p tsconfig.json) added to both packages, wired into the existing contracts-lint job — which already runs yarn build, and the check needs the generated typechain/ to exist.

Being precise about what this enforces: neither tsconfig sets strict, so zero is zero under the settings the packages already use. random-beacon additionally has noImplicitAny; ecdsa does not, and turning it on there would surface 112 errors (27 in typechain 6 output, 85 hand-written). Raising ecdsa to typechain 8 and then to noImplicitAny is worth doing, separately.

Also ignores .yarn/install-state.gz in ecdsa. yarn install drops 1 MB there and it shows up untracked; random-beacon already ignores it, and the root rule does not apply because a pattern containing a slash is anchored to its own directory.

Verification

Both suites run on the branch, unchanged from before it:

  • random-beacon: 955 passing, 0 failing
  • ecdsa: 673 passing, 0 failing
  • tsc --noEmit: 0 errors in both packages

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5d35f478-b9d1-4170-80c1-68ebe41d9436

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/solidity-typecheck-gate

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

mswilkison and others added 4 commits August 13, 2026 20:36
`random-beacon/tsconfig.json` sets `noImplicitAny: true`, but typechain 7
emits its ABI literals as bare `const _abi = [{ inputs: [], ... }]`. Under
that flag an empty array literal infers `any[]`, so every factory the
generator produced raised TS7018 -- 21 errors, all of them in generated
code that no one can edit. typechain 8 emits the same literals `as const`
and the whole class disappears.

The alternative was dropping `noImplicitAny`, which would have bought a
clean run by checking our own code less. This keeps the flag.

`@typechain/hardhat` is pinned to ^7 rather than the current ^9 because
9 defaults to the `ethers-v6` target and this package is still on v5.

The bump changes the output layout: v8 nests modules under their source
path instead of emitting flat files, so five deep imports move. Four are
Solidity library namespaces (`Groups`, `BeaconDkg`), which still have to
come from the module of a contract that uses them; the fifth,
`RandomBeaconGovernance`, is an ordinary contract type and now comes from
the barrel with its neighbours.

`yarn test` is unchanged at 955 passing, 0 failing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`submitRelayEntry` takes `bytes`, and the test handed it a `BigNumber`
built from 32 random bytes, which `tsc` rejects as not `BytesLike`.

It was not only a typing problem. ethers renders a `BigNumber` argument
through `toHexString()`, which drops leading zero bytes, so about one run
in 256 submitted an entry shorter than 32 bytes. The stub only
keccak-hashes the value, so nothing failed and nothing noticed.

The random bytes are a `Uint8Array`, already `BytesLike`, so they go in
directly.

`yarn test` is unchanged at 673 passing, 0 failing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing in CI ran the type checker. `yarn lint` is eslint, and hardhat
transpiles tests without checking them, so a type error reached main only
if someone happened to run `tsc` by hand. Both packages are at zero
errors as of the two commits before this one, and this keeps them there.

The step goes in `contracts-lint`, which already runs `yarn build` --
`typecheck` needs the generated `typechain/` to exist.

Worth being precise about what this enforces: neither tsconfig sets
`strict`, so zero is zero under the settings the packages already use.
random-beacon additionally has `noImplicitAny`; ecdsa does not, and
turning it on there would surface 112 errors (27 in typechain 6 output,
85 in hand-written code). Raising ecdsa to typechain 8 and then to
`noImplicitAny` is worth doing separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`yarn install` in this package drops a 1 MB `.yarn/install-state.gz` that
shows up as untracked and is easy to sweep into a commit. random-beacon
already ignores it; the root `.gitignore` rule does not apply here
because a pattern containing a slash is anchored to its own directory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@piotr-roslaniec
piotr-roslaniec force-pushed the chore/solidity-typescript-5 branch from 0f869ea to 0072a3c Compare August 13, 2026 18:36
@piotr-roslaniec
piotr-roslaniec force-pushed the chore/solidity-typecheck-gate branch from b8682b4 to b8e61f1 Compare August 13, 2026 18:36
@piotr-roslaniec
piotr-roslaniec removed this pull request from stack #4234 September 12, 2026 09:19
Base automatically changed from chore/solidity-typescript-5 to dev September 12, 2026 09:21
@piotr-roslaniec
piotr-roslaniec merged commit b8c0366 into dev Sep 12, 2026
31 checks passed
@piotr-roslaniec
piotr-roslaniec deleted the chore/solidity-typecheck-gate branch September 12, 2026 09:23
piotr-roslaniec added a commit that referenced this pull request Sep 12, 2026
Stacked on #4207.

**6.0.3 is the last TypeScript release with a JavaScript API.** I
checked what 7.x actually ships:

```
node_modules/typescript/lib/  →  getExePath.js  tsc.js  version.cjs
require("typescript").transpileModule       → undefined
require("typescript").createProgram         → undefined
require("typescript").createLanguageService → undefined
```

There is no `lib/typescript.js` — the 7.x package is a launcher for the
Go binary. ts-node (which hardhat 2 uses to run `hardhat.config.ts` and
the tests), `@typescript-eslint`, typechain and prettier's TS parser all
drive that API, so installing 7 today would break the build, the linter
and codegen at once. 6.x is where this stops until those ship tsgo
support.

## Three defaults changed, all pinned back

This is a version bump, not a tightening, so each new default is pinned
to what the packages already do:

| Default | Effect if left alone | Pinned to |
| --- | --- | --- |
| `strict` now on | strict debt in both packages | `"strict": false`
(random-beacon keeps its existing `noImplicitAny`) |
| `@types/*` no longer auto-included | mocha/chai globals vanish — **1
732 diagnostics in ecdsa alone** | `"types": [...]` naming the packages
that contribute globals |
| `rootDir` required for multi-directory projects | hardhat refuses to
start | `"rootDir": "."` |

The `rootDir` one is the reason to run more than the type checker: `tsc
--noEmit` stays perfectly green while `hardhat test` dies with `TS5011`.

Turning `strict` on is real work and belongs in its own PR.

## The ES5 export bundle is on borrowed time

Both packages publish an `export/` bundle built through
`tsconfig.export.json` at **`target: es5`**. In TypeScript 6, *both*
that target (`TS5107`) and `downlevelIteration` (`TS5101`) are
deprecated, and **both are removed in TypeScript 7**.

They are silenced here with `"ignoreDeprecations": "6.0"` rather than
removed, because at `target: es5` `downlevelIteration` genuinely changes
emit for `for…of` over non-array iterables, and that bundle is consumed
by downstream packages. In `ecdsa` this also matters for `prepare`,
which runs the same build on install — so getting it wrong breaks `yarn
install`, not just publishing.

**Raising the export target off ES5 is a prerequisite for TypeScript 7**
and is tracked separately.

## Verification

Both packages, on 6.0.3:

| | random-beacon | ecdsa |
| --- | --- | --- |
| `tsc --noEmit` | 0 errors | 0 errors |
| `tsc -p tsconfig.export.json` | builds | builds |
| `yarn lint` | 0 errors | 0 errors |
| `yarn test` | **955 passing / 0 failing** | **673 passing / 0
failing** |

Lockfile churn is 18 lines per package, all `typescript`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants